Skip to content

Bump protobuf bazel_dep 33.4 -> 33.6 (refs #339)#355

Closed
tinder-maxwellelliott wants to merge 1 commit into
masterfrom
claude/upgrade-protobuf-issue-339
Closed

Bump protobuf bazel_dep 33.4 -> 33.6 (refs #339)#355
tinder-maxwellelliott wants to merge 1 commit into
masterfrom
claude/upgrade-protobuf-issue-339

Conversation

@tinder-maxwellelliott
Copy link
Copy Markdown
Collaborator

@tinder-maxwellelliott tinder-maxwellelliott commented May 18, 2026

Summary

Refs #339. Bumps the bzlmod protobuf dep from 33.4 to 33.6, the latest stable release on the 33.x line. The Java runtime used by the deploy jar's java_proto_library targets ([@protobuf]) tracks the module version directly, so this is the single source of truth for what ships inside bazel-diff.jar — there is no separate Maven dep to update (we don't pull com.google.protobuf:protobuf-java from maven_install.json).

Why not 34.x

I first pushed this PR targeting protobuf 34.1 (latest stable on BCR). CI's test-jre11-run-example matrix runs against Bazel 7.x and failed analysis with:

Error: _create_proto_info() got unexpected keyword arguments:
  option_deps, extension_declarations

protobuf 34.0 began passing option_deps and extension_declarations to ProtoInfo(...), and Bazel 7.x's builtin ProtoInfo provider does not accept those. That's a hard incompatibility — 34.x requires Bazel 8+. Staying on the 33.x line preserves the existing Bazel 7.x CI matrix; a follow-up PR that drops 7.x from CI can move us to 34.x (and then to whatever lands the Unsafe migration).

Honest disclosure: does this fix the Unsafe warning?

No, not on its own. I verified by grepping the bytes of the freshly-built bazel-bin/cli/bazel-diff_deploy.jar:

$ unzip -p bazel-bin/cli/bazel-diff_deploy.jar com/google/protobuf/UnsafeUtil.class | grep -ao 'arrayBaseOffset\|sun/misc/Unsafe' | sort -u
arrayBaseOffset
sun/misc/Unsafe

protobuf v33.6's UnsafeUtil$MemoryAccessor.arrayBaseOffset still calls sun.misc.Unsafe.arrayBaseOffset(Class) directly — the same call site producing the JDK 24+ "terminally deprecated method" warning in #339. I also checked the protobuf main branch source: the call is still there, unchanged. v34.1 has it too.

The migration off sun.misc.Unsafe (likely to VarHandle) is tracked upstream at protocolbuffers/protobuf#20760. It has not landed at any released tag yet.

Bumping to 33.6 keeps us on the latest 33.x release so we'll pick up smaller fixes in this line. We'll pick up the Unsafe migration in a subsequent bump once it ships upstream and we move off Bazel 7.x.

I'd recommend leaving #339 open until upstream lands the fix and we bump again, rather than closing on this PR.

Verification

  • bazel build //cli:bazel-diff_deploy.jar — succeeds with protobuf 33.6.
  • bazel test over the 15 cli unit-test targets — all pass:
    • BazelClientTest, BazelModServiceTest, BazelRuleTest, BuildGraphHasherTest
    • CalculateImpactedTargetsInteractorTest, CalculateImpactedTargetsInteractorIssue335Test, ContentHashProviderTest
    • DeserialiseHashesInteractorTest, ModuleGraphParserTest
    • NormalisingPathConverterTest, OptionsConverterTest
    • ProcessStdinHangRegressionTest, SourceFileHasherTest
    • StderrPollutionRegressionTest, TargetHashTest
  • E2E tests not run locally due to a pre-existing JDK-env sandbox issue on this machine (Unable to locate a Java Runtime inside the nested bazel server, present on master without these changes). CI will run them.

Test plan

  • bazel build //cli:bazel-diff_deploy.jar passes.
  • All 15 cli unit-test targets pass under bazel test.
  • First attempt at 34.1 failed CI on Bazel 7.x; downgraded target to 33.6 and pushed.
  • CI green across the full matrix.
  • Once protocolbuffers/protobuf#20760 ships a fix, follow-up PR to bump (likely past 34.x, requires dropping Bazel 7.x first) and verify the warning is gone.

🤖 Generated with Claude Code

Pulls in the latest 33.x BCR-released protobuf module. The Java runtime
used by the deploy jar's `java_proto_library` targets ([@protobuf])
tracks the module version directly, so this is the single source of
truth for what ships inside bazel-diff.jar -- there is no separate
Maven dep to update.

Why not 34.x: protobuf 34.0 began passing `option_deps` and
`extension_declarations` to `ProtoInfo(...)`, which Bazel 7.x's builtin
`ProtoInfo` provider does not accept. CI test-jre11-run-example matrix
runs against Bazel 7.x and fails analysis with
`_create_proto_info() got unexpected keyword arguments: option_deps,
extension_declarations`. Staying on the 33.x line preserves Bazel 7.x
support; a follow-up PR that drops 7.x from CI can move to 34.x.

Known limitation: protobuf 33.6's `UnsafeUtil` still calls
`sun.misc.Unsafe.arrayBaseOffset` (verified by grepping the bytecode of
the freshly-built `bazel-bin/cli/bazel-diff_deploy.jar`), so the JDK 24+
"terminally deprecated method" warning reported in
#339 will continue to print
until upstream protobuf migrates `MemoryAccessor` to `VarHandle`. That
migration is tracked at
protocolbuffers/protobuf#20760 and has not
landed at any released tag (verified against protobuf `main` branch).

Verified with `bazel test` on the 15 cli unit-test targets; all pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tinder-maxwellelliott tinder-maxwellelliott force-pushed the claude/upgrade-protobuf-issue-339 branch from fb6b83c to d11d7f8 Compare May 18, 2026 19:00
@tinder-maxwellelliott tinder-maxwellelliott changed the title Bump protobuf bazel_dep 33.4 -> 34.1 (refs #339) Bump protobuf bazel_dep 33.4 -> 33.6 (refs #339) May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant